Add automatic corepack support for node v14 +#482
Add automatic corepack support for node v14 +#482Ethan-Arrowood wants to merge 4 commits intoactions:mainfrom
Conversation
|
Struggling to get a test to pass. Insight appreciated |
|
Probably wrong on the test, but since it is asserting the desired corepack output I'm marking this PR ready for review. Hopefully a maintainer more familiar with the test suite can provide some input on how to improve the tests. |
|
Thanks for opening this PR. I support the idea. |
styfle
left a comment
styfle
left a comment
There was a problem hiding this comment.
We also need to consider caching the result of corepack so that it doesn't need to install the package manger each time.
|
Added a check for the I almost want to try writing an integration test for this, but would appreciate some input from the maintainers |
There was a problem hiding this comment.
I'm in favor of using Corepack, but have some concerns about the implementation:
This doesn't seem to support the@versionsyntax, which is automatically used in Yarn 3.1, and documented as experimental in Node. We'd have to at least allow for this syntax to set the package manager properly, though it would be nice to also parse the version and use it by default.- GitHub runners already have Yarn installed. You're technically supposed to uninstall Yarn before enabling Corepack, as the global Yarn binary would conflict with Corepack's Yarn shim. We could try to fix this by either uninstalling Yarn before enabling Corepack, or removing Yarn from GitHub's runner (which would be a breaking change but would standardize behavior in the long run).
|
Oh sorry, I see what you mean. It's not actually parsing the
GitHub Actions seems to run |
|
@Ethan-Arrowood Are you planning to update this PR based on the feedback that has been provided? I know a lot of people are still interested in this feature. I see that CI is also failing currently. |
|
Looking at this PR again, it could be considered semver major because it will suddenly opt into corepack when the package.json is detected. I'm starting to think that PR #651 is better solution because it could be considered semver minor due to the manual opt in. That could prove valuable if anything changes in the future, since corepack is still technically experimental. |
* docs: update README.md [skip ci] * docs: update .all-contributorsrc [skip ci] --------- Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>
7 participants
Description:
Executes
corepack enablewhen the detected node version is 14+. This will enable support for thepackageManagerfield foryarnandpnpmRelated issue:
#480
Check list: